fix(web-components): preserve radio state after deferred upgrade by KirtiRamchandani · Pull Request #36243 · microsoft/fluentui

KirtiRamchandani · 2026-05-25T03:23:34Z

Previous Behavior

Parent components could collect matching child tags before those child custom elements had upgraded. When the parent then wrote child state, those writes could create own properties on bare HTMLElement instances and shadow the eventual component accessors.

The reported RadioGroup case created an own checked property on pending radios, but the same parent/child collection pattern exists across Accordion/AccordionItem, Listbox/Option, MenuList/MenuItem, and Tree/TreeItem.

New Behavior

Parent collection now goes through a shared upgraded-custom-element helper. Matching child tags are ignored until FAST has upgraded them, and the parent refreshes its collection once any matching pending tag definitions resolve.

This is wired into RadioGroup, Accordion, Listbox, MenuList, Tree, and TreeItem. Listbox also reapplies its current multiple state when options are collected after upgrade.

The regression fixture forces parent tags to upgrade before child tags and verifies:

RadioGroup preserves checked state without an own checked shadow property.
Listbox applies multiple to options after they upgrade without an own multiple shadow property.
Tree applies size to tree items after they upgrade without an own size shadow property.

Tests run

corepack yarn type-check from packages/web-components
corepack yarn eslint src/accordion/accordion.ts src/listbox/listbox.ts src/listbox/listbox.spec.ts src/menu-list/menu-list.base.ts src/radio-group/radio-group.base.ts src/tree/tree.base.ts src/tree/tree.spec.ts src/tree-item/tree-item.base.ts src/utils/custom-elements.ts test/src/parent-child-upgrade-order.ts from packages/web-components
corepack yarn e2e src/radio-group/radio-group.spec.ts src/listbox/listbox.spec.ts src/tree/tree.spec.ts --project=chromium -g "upgrade order" from packages/web-components
corepack yarn e2e src/accordion/accordion.spec.ts src/listbox/listbox.spec.ts src/menu-list/menu-list.spec.ts src/radio-group/radio-group.spec.ts src/tree/tree.spec.ts --project=chromium --workers=1 from packages/web-components
corepack yarn prettier --check packages/web-components/src/accordion/accordion.ts packages/web-components/src/listbox/listbox.ts packages/web-components/src/listbox/listbox.spec.ts packages/web-components/src/menu-list/menu-list.base.ts packages/web-components/src/radio-group/radio-group.base.ts packages/web-components/src/tree/tree.base.ts packages/web-components/src/tree/tree.spec.ts packages/web-components/src/tree-item/tree-item.base.ts packages/web-components/src/utils/custom-elements.ts packages/web-components/test/parent-child-upgrade-order.html packages/web-components/test/src/parent-child-upgrade-order.ts
corepack yarn check:change
git diff --check

Related Issue(s)

Fixes [Bug]: Radio Group Pre-Upgrade Property Shadowing Breaks checked State #36239

chrisdholt · 2026-05-25T17:32:18Z

Tagging @radium-v as a required reviewer.

radium-v

While I think this is pointing out a few important scenarios (tag names can be a fragile identifier; ancestral relationships between components all have additional structural concerns), I'd prefer if we could address the issue all-up. We have similar problems with Accordion/AccordionItem, Dropdown/Listbox/DropdownOption, MenuButton/Menu/MenuList/MenuItem, Tree/TreeItem, and others.

radium-v · 2026-05-26T16:13:13Z

    await expect(radios.nth(2)).toHaveJSProperty('checked', false);
  });

+  test('should preserve checked state when radios upgrade after the group', async ({ page }) => {


This test needs to be written using fastPage, but may need to be its own test fixture entrypoint. Doing it this way may allow us to reuse this for other components that have similar ancestor/descendant patterns.

radium-v · 2026-05-26T16:16:38Z

+  private static isUpgradedRadio(element: Element): element is Radio {
+    return element.localName.endsWith('-radio') && '$fastController' in element;
+  }


radio.options.ts already provides an isRadio predicate function that handles the tagname checking, and the $fastController check can be done in-place.

radium-v · 2026-05-26T16:25:00Z

-    this.radios = [...this.querySelectorAll('*')].filter(x => isRadio(x)) as Radio[];
+    Updates.enqueue(() => {
+      const elements = [...this.querySelectorAll('*')];
+      this.radios = this.getRadioDescendants();


Why get elements first and then get radio descendants separately? They both end up querying for all elements.

radium-v · 2026-05-26T16:30:05Z

+      const elements = [...this.querySelectorAll('*')];
+      this.radios = this.getRadioDescendants();
+
+      for (const tagName of new Set(


This is a weird pattern since Set provides iterator methods

radium-v · 2026-05-26T16:32:34Z

+        elements
+          .filter(x => x.localName.endsWith('-radio') && !BaseRadioGroup.isUpgradedRadio(x))
+          .map(x => x.localName),
+      )) {
+        customElements.whenDefined(tagName).then(() => {
+          if (this.isConnected) {
+            this.radios = this.getRadioDescendants();
+          }
+        });
+      }


There's potential for a lot of repetitive and unnecessary DOM querying going on here

radium-v · 2026-05-26T16:44:22Z

 import type { RadioGroup } from './radio-group.js';
 import { tagName } from './radio-group.options.js';

+const sourceBaseUrl = `/@fs${new URL('../', import.meta.url).pathname}`;


I'd rather we didn't rely on internal vite-specific constructs, or try to work outside of the bounds of the environment. I see custom pathing like this as a fragile workaround for something that can probably be handled in a better and more purposeful way. For instance, the new test can likely be its own test entrypoint.

KirtiRamchandani · 2026-05-31T07:31:38Z

Thanks for the review. I pushed a follow-up that narrows the implementation and removes the fragile test setup:

Reused the existing isRadio(...) predicate and kept the $fastController check local to the collection pass.
Avoided the extra descendant query by deriving upgraded and pending radios from the same element scan.
Replaced the inline /@fs imports with a dedicated upgrade-order fixture entrypoint under test/, while keeping the Playwright assertion in the radio-group spec via fastPage.
Batched pending radio definitions with one Promise.all(...) refresh instead of scheduling one query per tag.

Checks run:

corepack yarn type-check
corepack yarn eslint src/radio-group/radio-group.base.ts src/radio-group/radio-group.spec.ts test/src/radio-group-upgrade-order.ts
corepack yarn prettier --check packages/web-components/src/radio-group/radio-group.base.ts packages/web-components/src/radio-group/radio-group.spec.ts packages/web-components/test/radio-group-upgrade-order.html packages/web-components/test/src/radio-group-upgrade-order.ts
corepack yarn e2e src/radio-group/radio-group.spec.ts --project=chromium -g "preserve checked state"
corepack yarn e2e src/radio-group/radio-group.spec.ts --project=chromium --workers=1

KirtiRamchandani · 2026-06-02T07:32:32Z

Thanks for the direction here. I pushed a follow-up that moves this from the radio-only path to a shared upgraded-child collection helper and wires it into the parent/child paths called out in the review:

RadioGroup / Radio
Accordion / AccordionItem
Listbox / Option
MenuList / MenuItem
Tree / TreeItem

The helper only returns matching child custom elements after FAST has upgraded them, then refreshes the parent collection once matching pending tag definitions resolve. I also added parent-before-child upgrade-order coverage for RadioGroup, Listbox, and Tree so the regression checks both the collection refresh and the absence of own shadowing properties.

Validation run:

corepack yarn type-check
targeted ESLint on touched files
focused Chromium upgrade-order e2e slice (3 passed)
affected Accordion/Listbox/MenuList/RadioGroup/Tree Chromium specs (109 passed)
Prettier check
corepack yarn check:change
git diff --check

fix: preserve radio state after deferred upgrade

29db4f2

KirtiRamchandani requested a review from a team as a code owner May 25, 2026 03:23

chrisdholt requested a review from radium-v May 25, 2026 17:31

radium-v requested changes May 26, 2026

View reviewed changes

microsoft-github-policy-service Bot added the Needs: Author Feedback label May 26, 2026

fix(web-components): refine radio upgrade handling

633eade

microsoft-github-policy-service Bot removed the Needs: Author Feedback label May 31, 2026

fix(web-components): defer child collection until upgrade

16da9b1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(web-components): preserve radio state after deferred upgrade#36243

fix(web-components): preserve radio state after deferred upgrade#36243
KirtiRamchandani wants to merge 3 commits into
microsoft:masterfrom
KirtiRamchandani:fix/radio-group-upgrade-shadowing

KirtiRamchandani commented May 25, 2026 •

edited

Loading

Uh oh!

chrisdholt commented May 25, 2026

Uh oh!

radium-v left a comment

Uh oh!

radium-v May 26, 2026

Uh oh!

radium-v May 26, 2026

Uh oh!

radium-v May 26, 2026

Uh oh!

radium-v May 26, 2026

Uh oh!

radium-v May 26, 2026

Uh oh!

radium-v May 26, 2026

Uh oh!

KirtiRamchandani commented May 31, 2026

Uh oh!

KirtiRamchandani commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

KirtiRamchandani commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Previous Behavior

New Behavior

Tests run

Related Issue(s)

Uh oh!

chrisdholt commented May 25, 2026

Uh oh!

radium-v left a comment

Choose a reason for hiding this comment

Uh oh!

radium-v May 26, 2026

Choose a reason for hiding this comment

Uh oh!

radium-v May 26, 2026

Choose a reason for hiding this comment

Uh oh!

radium-v May 26, 2026

Choose a reason for hiding this comment

Uh oh!

radium-v May 26, 2026

Choose a reason for hiding this comment

Uh oh!

radium-v May 26, 2026

Choose a reason for hiding this comment

Uh oh!

radium-v May 26, 2026

Choose a reason for hiding this comment

Uh oh!

KirtiRamchandani commented May 31, 2026

Uh oh!

KirtiRamchandani commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KirtiRamchandani commented May 25, 2026 •

edited

Loading